-
-
Notifications
You must be signed in to change notification settings - Fork 690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix error reporting in view macro #2289
Conversation
Looks like after adding a bunch of spans |
Simply going on the basis of the screenshots: Wow. I have not looked through it in more depth yet, but just wanted to express my gratitude for your work on this, which will be a huge help to the whole community. |
@gbj Thank you for such kind words :3. That's actually just a starting point - I plan on making at least 2 more PRs: better support for LSP features inside On the side node: I am a bit of a dumby and forgot to run |
Thanks! I've just looked through and this looks great over all. However, it looks like there are still some failing tests in |
356a224
to
6bf2897
Compare
As per this discussion on discord I decided to remove snapshot tests for
|
What is the state/progress of this PR? Is there anything that should be done from my side? |
My apologies! The status of the PR is that I could've sworn it was merged long ago because it was excellent, when in reality I lost track of it during a busy month and have done nothing. I just merged the one outstanding conflict and will let the CI run now and merge. Thanks so much for the contribution and sorry about the delay. |
rust-analyzer: 2024-01-29 (v0.3.1823)
This PR is aimed to improve DX of using leptos by fixing error reporting in
view!
macro, so that errors inside it are reported over the token, that caused an error, instead of the wholeview!
macro call site.Note
This PR is tied to bugfix in rust-analyzer (that is already merged), so "minimum expected rust-analyzer version" is
v0.3.1823
. Also, in order to get better rust-analyzer feedback - you need to add"rust-analyzer.cargo.allFeatures": true
to your VSCode config (or similar flag for other IDEs). Should I add these two points toleptos-rs/book/getting_started/leptos_dx.md
?Here is a simple example:
List of all errors that are covered by this PR
unreachable_code
/missing_field
Disable "unreachable_code" lint when there is an error
view!
macroReport "missing field" and other
.build()
errors on component's nameRegression: "Go to Definition" triggered on
Component
name inview!
macro now suggestsbuild
method definition in addition toComponent
itself. Previously onlyComponent
was reportedcomponent_with_props_without_macro
Component with props but without
#[component]
macro is used inview!
macrogenerics
When incorrect generics are passed or inferred in component the error is reported over the whole
view!
macro.Regression: "Go to Definition" triggered on
Component
name inview!
macro now suggestsleptos::component_props_builder
definition in addition toComponent
itself.prop_into
Error reporting with
#[prop(into)]
events
Error for "on:" events should be reported on it's name
directive
Missing or incorrect type of the param when using directive
block_in_fragment
If block inside fragment returns something, that is not "renderable"
slots_prop_into
Error reporting with
#[prop(into)]
in slotswrong_slot_name
Wrong slot name
undefined_slot
Using slot that is not defined in a component
slot_expected_one_got_more
Expected one slot, got more
slot_unreachable_code
/slot_missing_field
Disable "unreachable_code" warn when there is an error in slot in
view!
macroReport "missing field" and other
.build()
errors on slot's nameRegression: "Go to Definition" triggered on
Then
inview!
macro now suggestsbuild
method definition in addition toThen
itself. Previously onlyThen
was reportedchildren_clone
Using
clone:
notation on type that does not implement clonechildren_let_expected_one_got_none
Expected
let:
binding, gotChildren
.children_projection_error
children_projection_error_slot_version
expected_children_got_let_bind
expected_children_got_let_bind_slot_version
expected_one_let_bind_got_more
expected_one_let_bind_got_more_slot_version
element_incorrect_attribute_type
element_incorrect_spread_type
element_incorrect_child_type
Note
There are also some regressions regarding "Go to Definition" LSP functionality - in some cases it now suggests implementation details alongside original definition. See
missing_field
,generics
andslot_missing_field
. Although it makes DX of using "Go to Definition" worse than before - I believe it's a fair trade-off for correct error reporting.Almost all examples listed above have a corresponding test case in
leptos_macro/tests/ui/view
. Some tests have subtly different output between ssr and client modes.Example
List of all problematic tests
Unfortunately, there is no easy way to implement these tests, because maintainer of
trybuild
crate refuses to allow specifying custom suffix onTestCases
: dtolnay/trybuild#245. Currently, I see 4 possible ways about this:view_ssr
andview_client
) and load them conditionally (based on current feature flags)trybuild
crate and add suffix functionalitytrybuild